JIT: Merge all RETURN/THROW blocks#128515
Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
Bot
added
the
community-contribution
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@AndyAyersMS PTAL. |
| // Avoid splitting a return away from a possible tail call | ||
| // | ||
| if (!block->hasSingleStmt()) | ||
| if (block->isEmpty()) |
There was a problem hiding this comment.
This check was here before, but I dont think we actually need it. Because we only accept RETURN or THROW blocks and these should never be empty?
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
Can we do this without repeatedly searching all blocks for returns and throws?
| do | ||
| { | ||
| predInfo.Reset(); | ||
| for (BasicBlock* const block : Blocks()) |
There was a problem hiding this comment.
The set of eligible return and throw blocks never changes, so do we need to repeatedly walk the entire block list here?
There was a problem hiding this comment.
I would also think we don't need to, however when I tried to hoist that it caused asserts.
I didn't look further into it because the same approach is already done in iterateTailMerge() and there are also multiple comments arround this code about improving algorithm efficiency.
So I'd prefer properly understanding the entire code and improving efficiency in a separate PR, in the future.
2 participants
Fix #128514
tailMergePreds(nullptr)was called once, but my understanding is it needs to be called repeatedly as it only processes one set at at time.